fixed bug that copy order incorrect at host component. #9426
Conversation
chihualee
commented
Sep 3, 2024
chihualee
commented
- fixed bug that copy order incorrect at host component.
- a lot of log just print wPtr and rPtr of local-buffer and dma-buffer, it can confirm copy order correctness. It should disable logs manually.
2. a lot of log just print wPtr and rPtr of local-buffer and dma-buffer, it can confirm copy order correctness. It should disable logs manually.
chihualee
requested review from
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
|
Can one of the admins verify this patch?
|
| @@ -194,6 +194,8 @@ static uint32_t host_get_copy_bytes_one_shot(struct host_data *hd, struct comp_d | |||
| */ | |||
There was a problem hiding this comment.
Please check existing git commits in this project for examples on how to write git commit. SOF follows similar practises as Linux kernel and Zephyr. One git commit per logical change, proper summary line, explains what bug you are fixing, etc
There was a problem hiding this comment.
please explain what order is incorrect. AFAICT you're only moving the dma_buffer_copy_to() for the one shot case and this will break normal copy.
There was a problem hiding this comment.
@ranj063
sorry, I am new to GitHub.
As mentioned in https://github.com/orgs/thesofproject/discussions/9362,
For capture stream scenario, I find a bug that copying order incorrect in Host component.
The copy order should be: Local-buffer => DMA-buffer => Host-buffer.
But actually it is first DMA-buffer => Host-buffer, then Local-buffer => DMA-buffer.
I think host_copy_one_shot() is correct for playback, but incorrect for capture.
So my patch is let host copy change into Local-buffer => DMA-buffer => Host-buffer:
Just move (copy from Local-buffer to DMA-buffer)
from host_common_update() to host_copy_one_shot().
There was a problem hiding this comment.
@kv2019i
sorry, I am new to GitHub.
I think we discuss bug and solution first.
Finally, if the patch is fine I will modify the commit log more normally.
| } else { | ||
| source = hd->local_buffer; | ||
| sink = hd->dma_buffer; | ||
| ret = dma_buffer_copy_to(source, sink, hd->process, bytes, DUMMY_CHMAP); | ||
| } |
There was a problem hiding this comment.
@chihualee any update, this part looks wrong as capture uses the DMA buffer for sink, unless your device has no DMA to copy data to host driver from DSP (i.e. you have shared memory) ?
There was a problem hiding this comment.
My capture mechanism is almost the same as original legacy-host,
first data from local buffer to DMA buffer,
then data from DMA buffer to Host buffer.
src/audio/host-legacy.c
Comment on lines -237 to 250
I think this part is doing that capture copying data from local buffer to DMA buffer.
I just move this part to src/audio/host-legacy.c
host_copy_one_shot() around at lines 214
Because it should do first.
